Skip to content

Allow JavaVariableLabelProvider's presentation to be overridden#639

Merged
iloveeclipse merged 2 commits intoeclipse-jdt:masterfrom
kysmith-csg:fix-internal-access
Mar 26, 2025
Merged

Allow JavaVariableLabelProvider's presentation to be overridden#639
iloveeclipse merged 2 commits intoeclipse-jdt:masterfrom
kysmith-csg:fix-internal-access

Conversation

@kysmith-csg
Copy link
Copy Markdown
Contributor

@kysmith-csg kysmith-csg commented Mar 13, 2025

This mostly reverts a4eae7f (#580) for this file, as well as some code cleanup. Instead of making the fLabelProvider final, we initialize it to a default value as before but provide getters/setters so the field can still be private. Details in #580 for why I need this change, but in short it's because our product has a debugger that is tightly coupled to the JDT debugger, basically being an extension of it. Since this probably only affects me, your attention is greatly appreciated.

I don't understand this code enough to know why fLabelProvider is static, given that label providers are not meant to be shared, so I left it alone. Otherwise it would make more sense to have it as an instance field, maybe defined in an overloaded constructor.

I also thought to update the version of this plugin to 3.15 but Eclipse gave me an error when I did that about "no new APIs added" which I don't think is correct, but please let me know.

Copy link
Copy Markdown
Member

@iloveeclipse iloveeclipse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I ask you to do following: first commit would add new non-API code (and have updated commit message), second commit would do a cleanup, also with appropriate message. For the second commit, could you please also remove all (non-Javadoc) comments on methods? Thanks.

@kysmith-csg kysmith-csg force-pushed the fix-internal-access branch from b6da226 to f9ce2ac Compare March 17, 2025 14:36
@kysmith-csg
Copy link
Copy Markdown
Contributor Author

@iloveeclipse done, I hope this is what you meant

@iloveeclipse iloveeclipse merged commit 62dd506 into eclipse-jdt:master Mar 26, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants